Add config store read path and split storage module (PR3)#548
Merged
Conversation
Rename crates/common → crates/trusted-server-core and crates/fastly → crates/trusted-server-adapter-fastly following the EdgeZero naming convention. Add EdgeZero workspace dependencies pinned to rev 170b74b. Update all references across docs, CI workflows, scripts, agent files, and configuration.
Introduces trusted-server-core::platform with PlatformConfigStore, PlatformSecretStore, PlatformKvStore, PlatformBackend, PlatformHttpClient, and PlatformGeo traits alongside ClientInfo, PlatformError, and RuntimeServices. Wires the Fastly adapter implementations and threads RuntimeServices into route_request. Moves GeoInfo to platform/types as platform-neutral data and adds geo_from_fastly for field mapping.
…o-pr2-platform-traits
- Defer KV store opening: replace early error return with a local UnavailableKvStore fallback so routes that do not need synthetic ID access succeed when the KV store is missing or temporarily unavailable - Use ConfigStore::try_open + try_get and SecretStore::try_get throughout FastlyPlatformConfigStore and FastlyPlatformSecretStore to honour the Result contract instead of panicking on open/lookup failure - Encapsulate RuntimeServices service fields as pub(crate) with public getter methods (config_store, secret_store, backend, http_client, geo) and a pub new() constructor; adapter updated to use new() - Reference #487 in FastlyPlatformHttpClient stub (PR 6 implements it) - Remove unused KvPage re-export from platform/mod.rs - Use super::KvHandle shorthand in RuntimeServices::kv_handle()
- Split fastly_storage.rs into storage/{config_store,secret_store,api_client,mod}.rs
- Add PlatformConfigStore read path via FastlyPlatformConfigStore::get using ConfigStore::try_open/try_get
- Add PlatformError::NotImplemented variant; stub write methods on FastlyPlatformConfigStore and FastlyPlatformSecretStore
- Add StoreName/StoreId newtypes with From<String>, From<&str>, AsRef<str>
- Add UnavailableKvStore to core platform module
- Add RuntimeServicesBuilder replacing 7-arg constructor
- Migrate get_active_jwks and handle_trusted_server_discovery to use &RuntimeServices
- Update call sites in signing.rs, rotation.rs, main.rs
- Add success-path test for handle_trusted_server_discovery using StubJwksConfigStore
- Fix test_parse_cookies_to_jar_empty typo (was emtpy)
Collaborator
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Review Summary
Well-structured PR — the storage module split is clean, the PlatformConfigStore read path is correctly implemented, and the migration to &RuntimeServices preserves error context properly. No blockers. CI is fully green.
Highlights:
- Strong newtype pattern for
StoreName/StoreIdprevents mix-up bugs RuntimeServicesBuilderwith exhaustiveexpect("should ...")messages- Graceful KV store degradation with
UnavailableKvStorefallback - Excellent
test_handle_trusted_server_discovery_returns_jwks_documenttest
Findings: 0 blockers, 2 high, 4 medium, 3 low
Findings placed in body (line not in diff)
🤔 [P2] Value not URL-encoded (pre-existing) — crates/trusted-server-core/src/storage/api_client.rs line 122
The payload format!("item_value={}", value) sends application/x-www-form-urlencoded content but doesn't actually URL-encode value. If value contains &, =, +, spaces, or JSON characters ({, }, "), the Fastly API may misinterpret it. This is pre-existing code (moved from fastly_storage.rs) but worth flagging since it's used in key rotation.
Consider: let payload = format!("item_value={}", urlencoding::encode(value));
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Make StoreName and StoreId inner fields private; From/AsRef provide all needed construction and access - Add #[deprecated] to GeoInfo::from_request with #[allow(deprecated)] at the three legacy call sites to track migration progress - Enumerate the six platform traits in the platform module doc comment - Extract backend_config_from_spec helper to remove duplicate BackendConfig construction in predict_name and ensure - Replace .into_iter().collect() with .to_vec() on secret plaintext bytes - Remove unused bytes dependency from trusted-server-adapter-fastly - Add comment on SecretStore::open clarifying it already returns Result (unlike ConfigStore::open which panics)
ChristianPavilonis
approved these changes
Mar 25, 2026
aram356
approved these changes
Mar 30, 2026
prk-Jr
changed the base branch from
main
to
feature/edgezero-pr2-platform-traits
prk-Jr
changed the title
aram356
approved these changes
Apr 1, 2026
prk-Jr
changed the base branch from
feature/edgezero-pr2-platform-traits
to
main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fastly_storage.rsintostorage/{config_store,secret_store,api_client,mod}.rsfor better separation of concernsPlatformConfigStoreread path in the Fastly adapter (FastlyPlatformConfigStore::getviaConfigStore::try_open/try_get)get_active_jwksandhandle_trusted_server_discoveryto use&RuntimeServicesinstead of the legacyFastlyConfigStoredirectlyChanges
crates/trusted-server-core/src/storage/mod.rsStoreName,StoreId,UnavailableKvStorecrates/trusted-server-core/src/storage/config_store.rsPlatformConfigStorestub with read support andNotImplementedwrite stubscrates/trusted-server-core/src/storage/secret_store.rsPlatformSecretStorewithNotImplementedwrite stubscrates/trusted-server-core/src/storage/api_client.rsfastly_storage.rs; retains API client helperscrates/trusted-server-core/src/fastly_storage.rsstorage/modulecrates/trusted-server-core/src/lib.rsstoragemodule; removefastly_storageexportcrates/trusted-server-core/src/platform/error.rsPlatformError::NotImplementedvariantcrates/trusted-server-core/src/platform/traits.rsNotImplementedon write methods in trait doc commentscrates/trusted-server-core/src/platform/types.rsStoreName/StoreIdnewtypes; addUnavailableKvStore; addRuntimeServicesBuildercrates/trusted-server-adapter-fastly/src/platform.rsFastlyPlatformConfigStore::get; stub write methods on config/secret store implscrates/trusted-server-adapter-fastly/src/main.rsRuntimeServicesBuilder; update import paths after storage module renamecrates/trusted-server-core/src/request_signing/jwks.rsget_active_jwksto accept&RuntimeServicescrates/trusted-server-core/src/request_signing/endpoints.rshandle_trusted_server_discoveryto accept&RuntimeServices; add success-path test usingStubJwksConfigStorecrates/trusted-server-core/src/request_signing/rotation.rscrates/trusted-server-core/src/request_signing/signing.rsCloses
Closes #484
Test plan
cargo test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/js/lib && npx vitest runcd crates/js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1Checklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)